chore: Address linting issues #12851
Conversation
faddat
changed the title
github-actions
bot
added
C:Cosmovisor
github-actions
bot
added
C:CLI
C:Keys
|
Awesome work and thank you!!! |
faddat
changed the title
CHANGELOG.md
Outdated
| @@ -45,6 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ | |||
|
|
|||
| ### Improvements | |||
|
|
|||
| * (ci) [#12851](https://github.com/cosmos/cosmos-sdk/pull/12851) lint grandfathered issues | |||
cosmovisor/go.mod
Outdated
| @@ -3,7 +3,7 @@ module github.com/cosmos/cosmos-sdk/cosmovisor | |||
| go 1.18 | |||
|
|
|||
| require ( | |||
| github.com/cosmos/cosmos-sdk v0.46.0 | |||
| github.com/cosmos/cosmos-sdk v0.46.0-beta2.0.20220808215643-b6791b7af44b | |||
There was a problem hiding this comment.
ah, I see. This can cause a thing....
Will take me a moment to figure out.
There was a problem hiding this comment.
Yeah this is because you did go work sync after having added cosmovisor in the go.work
There was a problem hiding this comment.
@faddat, meaculpa, removing this was breaking make test. We have a few solutions to fix that:
- In
run-tests(Makefile), addGOWORK=offbefore runninggo test. This requires (although I think it's good), packages to be properly tagged to pass CI. - Accept that it will upgrade the tendermint version until the SDK is downgrading to 34.x on main as well (as the support of 0.35 is anyway dropped by TM)
There was a problem hiding this comment.
I am actually for doing both, merging with cosmovisor in the go.work but then still disable it in the CI runs to force us to tag properly our versions. Wdyt @faddat, @marbar3778?
| @@ -5,6 +5,7 @@ use ( | |||
| ./api | |||
| ./client/v2 | |||
| ./core | |||
| ./cosmovisor | |||
There was a problem hiding this comment.
I think we decided to remove this? @julienrbrt you did something related to this no?
There was a problem hiding this comment.
It will likely need to be re-removed, then its removal will cause some kind of issue I don't fully understand.
There was a problem hiding this comment.
Your issue was in go 1.19 PR you said right? So it won't happen in this PR anyway :)
| dp := v1.NewDepositParams(params.MinDeposit, params.MaxDepositPeriod) | ||
| vp := v1.NewVotingParams(params.VotingPeriod) | ||
| tp := v1.NewTallyParams(params.Quorum, params.Threshold, params.VetoThreshold) | ||
| dp := v1.NewDepositParams(params.MinDeposit, params.MaxDepositPeriod) //nolint:staticcheck // TODO: refactor to only refer to Params |
There was a problem hiding this comment.
wasn't sure how to fix-- also if to fix.
Is fix simply getting rid of everything that works like params.Thingy ?
There was a problem hiding this comment.
(or, is it just wrong, and the todos aren't needed?)
|
if this passes 00 and 01 I'd say that it is ready. also, the diff between the most recent commit and the one before might be interesting, as I don't think that anything that was changed, should have affected those tests. |
|
That did not work. let's see where this one leaves us-- it reverts changes to snapshots. I am flummoxed. |
|
Apologies. I think that there is something deficient in this branch, and that it is most prudent to do-over from main. |
|
Superseded by #12867 |
Description
This PR addresses linting issues prior to go 1.19 usage.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change